Viewing the hmem settings as a global environment configuration, the best option I can think relates to fi_getparams(), fi_freeparams(), and fi_param_get(). Introducing fi_param_set() would allow a user to configure libfabric prior to calling fi_getinfo(), and the results would be global. This does mean that providers, including DL builds, need to be updated to check the values and adjust their implementation accordingly. The libfabric core may need to handle fi_param_set() being called prior to fi_param_define(); I'm uncertain on this. Providers will need to read the parameter, and probably cache the result, after fi_getinfo() has been called for the first time, so the app has time to call fi_param_set().

@shefty How is fi_param_set different from setting the environment variable directly? After we define these variables, how do we enforce application to programmatically set these values via fi_param_set?

fi_param_set() behaves the same as setting an environment variable directly. I doubt it ends up being any different than calling setenv(). The app will want to make the call to constrain the provider and allow things to work correctly. This isn't something libfabric forces on the app; it's the other way around.

Again, fi_info configures an instance of some object. That's not what's needed here. CUDA requires a global configuration.

If it works the same as setting the environment variable, why do we need to add fi_param_set()?

We can advise application which want to set such params programmatically via fi_param_set. I expect it can be overridden by the environment variable passed by user in the scripts (via export). @shefty does that match your expectation?

@jiaxiyan setting environment variable via export need user to modify their launch scripts which is what we try to avoid. using setenv directly inside program is not thread safe and programmer needs to do more things to make it correct.

@shijin-aws - Yes, I think that would work. I would usually/always defer to using an administrative setting (e.g. export) in case of conflicts. I don't know what the actual limitations are regarding multi-thread support with setenv(), except that it's documented as not being thread safe like you mentioned.

The libfabric core may need to handle fi_param_set() being called prior to fi_param_define(); I'm uncertain on this. Providers will need to read the parameter, and probably cache the result, after fi_getinfo() has been called for the first time, so the app has time to call fi_param_set().

@shefty Can we make fi_param_set call fi_ini to fi_param_define the new fields use_p2p, api_permitted, and use_dev_reg_copy in ofi_hmem_init before calling fi_getinfo?

I'm not following the question. Generically, I think the code just needs to handle fi_param_set() being called prior to whatever variable it's trying to modify may be defined. Consider that provider specific variables may be defined late, maybe not until the app calls fi_getinfo().

I'm picturing fi_param_set() aligning with fi_param_get().

To add confusion, there's another option for setting 'complex' configuration values. See fi_open(). That call could be extended to support "hmem". A user could do something like this:

/* in fi_ext.h */
struct hmem_fid {
    struct fid fid;
    struct fi_ops_hmem *ops
};
    
fi_open(.., "hmem", ..., &hmem_fid, ..);
hmem_fid->ops->set_attr(...)

(The syntax is incorrect.) hmem_fid would be a pointer to internal, global hmem settings. Once the user has the fid, we have the full range of the API, including custom ops. fi_open() is what's used to replace the mr_cache and logging components.

I don't know what the actual limitations are regarding multi-thread support with setenv(), except that it's documented as not being thread safe like you mentioned.

PyTorch has solved all its problems with threads, so during initialization there's a ton of PyTorch threads running around calling getenv(). Calling setenv() during that time can cause the environ pointer to be copied / freed, such that getenv() is walkng down a since-freed environ array. And then there is the inevitable segfault. This is not theoretical; it is significantly worse on platforms like Trainium where the default execution mode is 1 process with a thread group per accelerator compared to Nvidia's default of 1 process per accelerator, but it can happen any time the upper layer allows multiple threads during network initialization.

@shefty Thinking about it more, I agree with your statement

Conceptually, the application or an administrator is programming the provider implementation through some sort of configuration mechanism. That's typically been done using environment variables, or setop() when done programmatically. We don't want to link all these configuration values off of fi_info.

And most of the configurations we like to introduce in this PR is already covered by the option names in fi_setopt, like FI_OPT_FI_HMEM_P2P and FI_OPT_CUDA_API_PERMITTED. However, fi_setopt is a ep level operation and cannot cover the other fi resources under the domain, including MR.

I think the hmem related options can definitely be set in the domain level, but we don't have a setopt/getopt for domain. Do you think we can extend fi_setopt/fi_getopt to make it query/set contributions for both endpoint and domain levels, or introducing a domain specific setopt/getopt?

I am slightly inclined to make fi_setopt/fi_getopt to cover more levels than endpoint because we currently already have an enum

enum {
FI_OPT_ENDPOINT
}

that only covers endpoint level. And passing such enum to a ep-level only fi_setopt/fi_getopt seems redundant. WDYT?

Extending setopt to cover the domain isn't a problem. I don't have an issue with that.

But a domain level option is still an option set on the instance of some object. AFAICT, some of the hmem configuration settings need to be global. Can it realistically work if one domain uses CUDA calls, but another cannot? Again, consider the case where a single process accesses libfabric through multiple middleware libraries (e.g. NCCL, MPI, Mercury). Assuming we're not dealing with statically linked libraries (environment variables are the only option there), if one of the middleware components needs to disable CUDA, that needs to apply to all components.

It was probably a mistake to introduce FI_OPT_CUDA_API_PERMITTED, unless the app has some sort of weird way of dedicating GPUs to NICs that somehow makes this work.

Can it realistically work if one domain uses CUDA calls, but another cannot? Again, consider the case where a single process accesses libfabric through multiple middleware libraries (e.g. NCCL, MPI, Mercury). Assuming we're not dealing with statically linked libraries (environment variables are the only option there), if one of the middleware components needs to disable CUDA, that needs to apply to all components.

I don't think disabling CUDA is necessarily a global setting. For example, NCCL doesn't allow Libfabric to do cudaMemcpy to move data between bounce buffer and the cuda buffer that cuda buffer (passed by NCCL) may already be in a cudaStream so it doesn't want the network library to mess it up. Open MPI AFAIK doesn't do such stream things and it should permit cuda API inside Libfabric. Such different behavior depends on how application (MPI and NCCL) here manage their cuda buffer,

Similar example is for cuPointerSetAttribute, which is usually used to set some flags to a cuda pointer, e.g. enforce a synchronous memcpy from/to such cuda pointer. Libfabric or other communication libraries need to set such attribute to the passed in cuda buffer to ensure the data integrity. However, this attribute isn’t supported on memory was allocated using the VMM APIs (aka CUDA driver APIs). NCCL was using such cuda driver API to allocate the buffers passed to Libfabric, and then Libfabric cannot set such attribute then.

Such discrepancy on the CUDA memory management between different middlewares makes me believe this cuda api permitted shouldn't be a global setting. It may be special for Nvidia because of its complicated API family and history, but having such attribute set via domain or other libfabric resource gives us enough flexibility to handle similar issue in the future.

A domain setopt is only marginally different than an endpoint setopt, which already exists. If anything, an endpoint setopt is more flexible.

The larger problem is avoiding circular dependencies between the compute API and communication API. If a compute kernel is waiting on network data, that forms a compute -> communication dependency. If the communication library calls the compute API to complete the transfer, that can create a reverse communication -> compute dependency. Note that it doesn't matter if the compute API (e.g. CUDA) is called directly by the app or through middleware (e.g. NCCL), and the same is true of the communication API (e.g. direct calls to libfabric, libfabric via NCCL, libfabric via MPI).

It's possible such circular dependencies won't result in deadlock, but it's difficult to determine. For an app using middleware (NCCL, MPI), the app may not be able to guarantee it won't. Avoid deadlock in this case may require a big hammer that disables all communication -> compute dependencies.

AFAIK, a big hammer is required by NCCL, but would also work with all other cases, so I think we should design controlling this library wide. There's already a more refined option available. (I just doubt it can realistically work in practice).